refactor: modularize event loop node class methods and helpers by sundaram2021 · Pull Request #6633 · aden-hive/hive

sundaram2021 · 2026-03-19T03:40:56Z

Description

Modularizes EventLoopNode by extracting helper responsibilities into focused event_loop modules while preserving existing behavior and public compatibility. The orchestration flow remains in event_loop_node.py, and the extracted logic is wired back through thin delegation methods.

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Refactoring (no functional changes)

Related Issues

Fixes #6596

Changes Made

Extracted event loop helper logic into focused modules for compaction, cursor persistence, event publishing, judge evaluation, stall detection, synthetic tools, subagent execution, and tool result handling.
Updated EventLoopNode to delegate to the extracted modules while keeping the main orchestration logic in place.
Preserved compatibility for subagent escalation routing, compaction limits, output learning/spillover behavior, and existing imports/behavior expected by tests.

Testing

Describe the tests you ran to verify your changes:

Unit tests pass (cd core && pytest tests/)
Lint passes (cd core && ruff check .)
Manual testing performed

Targeted verification performed:

tests/test_event_loop_node.py
tests/test_event_loop_wiring.py
tests/test_node_conversation.py::TestIsContextTooLargeError
tests/test_node_conversation.py::TestLlmCompact
tests/test_subagent.py -k "EscalationReceiver or SubagentJudge or wait_for_response_registers_receiver_in_registry"
tests/test_subagent_escalation_e2e.py::test_escalation_cleanup_after_completion
ruff check framework/graph/event_loop_node.py framework/graph/event_loop

Checklist

My code follows the project's style guidelines
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Screenshots (if applicable)

Not applicable.

Extract EventLoopNode helper logic into focused event_loop modules while keeping the node responsible for orchestration. Preserve the existing behavior and compatibility for compaction, event publishing, cursor persistence, synthetic tools, judge evaluation, stall detection, tool result handling, and subagent escalation wiring.

Apply Ruff formatting to the extracted event loop modules, the EventLoopNode wrappers, and the OpenRouter key check script so the lint CI format check passes cleanly.

sundaram2021 · 2026-03-20T05:39:58Z

please review it and let me know if any changes needed

cc: @TimothyZhang7 @Hundao @bryanadenhq

sundaram2021 · 2026-03-23T10:11:54Z

hey @TimothyZhang7 please review this PR (assigned by yourself)
and let me know if there's any changes needed .
thanks

TimothyZhang7

Review

The decomposition direction is right — event_loop_node.py was genuinely too large, and the 8 submodule boundaries (compaction, cursor_persistence, event_publishing, judge_pipeline, stall_detector, subagent_executor, synthetic_tools, tool_result_handler) are cohesive. Backward compatibility is preserved. But there are structural issues that should be addressed before merge.

Circular import not resolved

judge_pipeline.py imports back from the parent it was extracted from:

# judge_pipeline.py
from framework.graph.event_loop_node import JudgeVerdict

JudgeVerdict should be moved to a shared types file (e.g. framework/graph/event_loop/types.py) so submodules don't depend on their parent.

`Any` used as a circular-import workaround

subagent_executor.py, compaction.py, and judge_pipeline.py type key parameters as Any with comments like # NodeContext, # LoopConfig, # OutputAccumulator. event_publishing.py shows this is avoidable — it imports EventBus and NodeContext directly. The others should use TYPE_CHECKING guards to get real types without runtime circular imports.

Callback injection as circular-import workaround

compaction.compact() takes build_message_inventory_fn, publish_context_usage_fn, and write_debug_log_fn as optional Callable parameters. This is dependency injection to dodge a circular import — better to fix the import structure so the function can call them directly.

Local imports inside delegation methods

def _build_ask_user_tool(self) -> Tool:
    from framework.graph.event_loop.synthetic_tools import build_ask_user_tool
    return build_ask_user_tool()

Method-level imports work but are a symptom of the same unresolved circular import architecture.

Minor

Em dash changed to hyphen in a user-visible return string (— description: → - description:) — a semantic change embedded in a refactor.
core/frontend/package-lock.json appears to be a spurious change unrelated to this refactor.
Several meaningful inline comments and docstrings were deleted from event_loop_node.py that documented non-obvious behavior.

The core issue across items 1–4 is the same: the import graph between event_loop_node.py and its submodules wasn't fully resolved, so the circular dependency was patched over rather than fixed. Resolving the JudgeVerdict circular dep and using TYPE_CHECKING imports would address most of it.

sundaram2021 · 2026-03-23T18:29:23Z

thanks for the review , I'm looking into these things

sundaram2021 · 2026-03-23T19:35:27Z

Review

The decomposition direction is right — event_loop_node.py was genuinely too large, and the 8 submodule boundaries (compaction, cursor_persistence, event_publishing, judge_pipeline, stall_detector, subagent_executor, synthetic_tools, tool_result_handler) are cohesive. Backward compatibility is preserved. But there are structural issues that should be addressed before merge.

Circular import not resolved

judge_pipeline.py imports back from the parent it was extracted from:
# judge_pipeline.py
from framework.graph.event_loop_node import JudgeVerdict
JudgeVerdict should be moved to a shared types file (e.g. framework/graph/event_loop/types.py) so submodules don't depend on their parent.

Any used as a circular-import workaround

subagent_executor.py, compaction.py, and judge_pipeline.py type key parameters as Any with comments like # NodeContext, # LoopConfig, # OutputAccumulator. event_publishing.py shows this is avoidable — it imports EventBus and NodeContext directly. The others should use TYPE_CHECKING guards to get real types without runtime circular imports.

Callback injection as circular-import workaround

compaction.compact() takes build_message_inventory_fn, publish_context_usage_fn, and write_debug_log_fn as optional Callable parameters. This is dependency injection to dodge a circular import — better to fix the import structure so the function can call them directly.

Local imports inside delegation methods
def _build_ask_user_tool(self) -> Tool:
    from framework.graph.event_loop.synthetic_tools import build_ask_user_tool
    return build_ask_user_tool()
Method-level imports work but are a symptom of the same unresolved circular import architecture.

Minor

Em dash changed to hyphen in a user-visible return string (— description: → - description:) — a semantic change embedded in a refactor.

core/frontend/package-lock.json appears to be a spurious change unrelated to this refactor.

Several meaningful inline comments and docstrings were deleted from event_loop_node.py that documented non-obvious behavior.

The core issue across items 1–4 is the same: the import graph between event_loop_node.py and its submodules wasn't fully resolved, so the circular dependency was patched over rather than fixed. Resolving the JudgeVerdict circular dep and using TYPE_CHECKING imports would address most of it.

I have addressed most of the issues . please review it and let me know if any further changes needed

cc: @TimothyZhang7 , thanks

levxn

Small fixes, address them, thanks!

levxn · 2026-03-25T20:27:08Z

@@ -81,28 +139,14 @@ async def _describe_images_as_text(image_content: list[dict[str, Any]]) -> str |
            if description:
                count = len(image_content)
                label = "image" if count == 1 else f"{count} images"
-                return f"[{label} attached — description: {description}]"
+                return f"[{label} attached \u2014 description: {description}]"


Please revert to keep this a clean refactor with no unintended diffs.

levxn · 2026-03-25T20:27:29Z

@@ -81,28 +139,14 @@ async def _describe_images_as_text(image_content: list[dict[str, Any]]) -> str |
            if description:
                count = len(image_content)
                label = "image" if count == 1 else f"{count} images"
-                return f"[{label} attached — description: {description}]"
+                return f"[{label} attached \u2014 description: {description}]"


Suggested change

return f"[{label} attached \u2014 description: {description}]"

return f"[{label} attached — description: {description}]"

levxn · 2026-03-25T20:28:36Z

These "peer": true additions are unrelated to this backend refactor.

levxn · 2026-03-25T20:50:10Z

+            char_limit=self._LLM_COMPACT_CHAR_LIMIT,
+            max_depth=self._LLM_COMPACT_MAX_DEPTH,
+            max_context_tokens=self._config.max_context_tokens,
+        )

    async def _llm_compact_split(


this function is a deadcode, can be removed, and also its relevant import in line 28.

__llm_compact_split is used in compaction.py line number 182
for summarising the messages when context limit hits

remove the dead _llm_compact_split import and delegation wrapper in ore/framework/graph/event_loop_node.py. The one you are using in compaction.py has been declared separately inside its respective file, check properly.

sundaram2021 · 2026-03-26T03:45:49Z

Small fixes, address them, thanks!

done

cc: @levxn

levxn

Please rebase off main cleanly and ensure only the EventLoopNode modularization changes are in this PR. As-is, this cannot be reviewed or merged safely.

sundaram2021 · 2026-03-26T19:25:59Z

Please rebase off main cleanly and ensure only the EventLoopNode modularization changes are in this PR. As-is, this cannot be reviewed or merged safely.

done , please look now
cc: @levxn

levxn

Good to merge, @TimothyZhang7 you can confirm from your end as well

TimothyZhang7

lgtm

sundaram2021 added 5 commits March 19, 2026 09:07

chore: refresh locks

a0b9d6a

Merge branch 'main' into refactor/event-loop-node-modularization

75a2ef2

style(repo): fix ruff format violations

fa7ce53

Apply Ruff formatting to the extracted event loop modules, the EventLoopNode wrappers, and the OpenRouter key check script so the lint CI format check passes cleanly.

fix: merge conflict

4885db3

sundaram2021 changed the title ~~Modularize event loop helpers~~ refactor: modularize event loop node class methods and helpers Mar 19, 2026

sundaram2021 added 2 commits March 20, 2026 10:01

chore: resolve merge conflict

49fd443

chore: pull latest change , refactor: modularize latest change

9b59255

chore: pull latest changes ; fix: merge conflict

ee4682c

TimothyZhang7 reviewed Mar 23, 2026

View reviewed changes

sundaram2021 added 3 commits March 24, 2026 00:59

refactor(event-loop): add shared event loop types

4724320

refactor(event-loop): untangle modular event loop imports

b43044c

refactor(event-loop): slim event loop node orchestration

d648f3d

sundaram2021 requested a review from TimothyZhang7 March 24, 2026 19:18

levxn suggested changes Mar 25, 2026

View reviewed changes

chore: revert changes

e188c26

sundaram2021 requested a review from levxn March 26, 2026 02:47

chore: pull latest changes

9667dd2

sundaram2021 force-pushed the refactor/event-loop-node-modularization branch from 1eac28d to 9667dd2 Compare March 26, 2026 16:27

levxn suggested changes Mar 26, 2026

View reviewed changes

sundaram2021 added 2 commits March 27, 2026 00:45

refactor: cleanup compact llm function

bf0b9a1

chore: pull latest change

eb4f180

sundaram2021 requested a review from levxn March 26, 2026 19:23

levxn reviewed Mar 26, 2026

View reviewed changes

TimothyZhang7 approved these changes Mar 26, 2026

View reviewed changes

TimothyZhang7 merged commit 38b79ed into aden-hive:main Mar 26, 2026
7 checks passed

sundaram2021 mentioned this pull request Apr 24, 2026

Vouch setup triggerdotdev/trigger.dev#3448

Open

	return f"[{label} attached \u2014 description: {description}]"
	return f"[{label} attached — description: {description}]"

Uh oh!

Conversation

sundaram2021 commented Mar 19, 2026

Description

Type of Change

Related Issues

Changes Made

Testing

Checklist

Screenshots (if applicable)

Uh oh!

sundaram2021 commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sundaram2021 commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TimothyZhang7 left a comment

Choose a reason for hiding this comment

Review

Circular import not resolved

Any used as a circular-import workaround

Callback injection as circular-import workaround

Local imports inside delegation methods

Minor

Uh oh!

sundaram2021 commented Mar 23, 2026

Uh oh!

sundaram2021 commented Mar 23, 2026

Review

Circular import not resolved

Any used as a circular-import workaround

Callback injection as circular-import workaround

Local imports inside delegation methods

Minor

Uh oh!

levxn left a comment

Choose a reason for hiding this comment

Uh oh!

levxn Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

levxn Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

sundaram2021 Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

levxn Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

sundaram2021 Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

levxn Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

sundaram2021 Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

levxn Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

sundaram2021 commented Mar 26, 2026

Uh oh!

levxn left a comment

Choose a reason for hiding this comment

Uh oh!

sundaram2021 commented Mar 26, 2026

Uh oh!

levxn left a comment

Choose a reason for hiding this comment

Uh oh!

TimothyZhang7 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

sundaram2021 commented Mar 20, 2026 •

edited

Loading

sundaram2021 commented Mar 23, 2026 •

edited

Loading

`Any` used as a circular-import workaround

`Any` used as a circular-import workaround

sundaram2021 Mar 26, 2026 •

edited

Loading